Enhancement: Run "pa app remove" as admin. Closes #5519#5610
Enhancement: Run "pa app remove" as admin. Closes #5519#5610
Conversation
|
Maybe related to this issue...
It seems to me that |
|
Thank you @MartinM85! We'll try to review it ASAP! |
Great catch @MartinM85, looks like two identical tests indeed. I think we can remove test |
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
Nice work so far @MartinM85! Let's change a few things before we proceed.
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
Command works great, let's change a few tiny things before we merge this.
| if (args.options.asAdmin) { | ||
| endpoint = `${this.resource}/providers/Microsoft.PowerApps/scopes/admin/environments/${formatting.encodeQueryParameter(args.options.environmentName!)}/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`; | ||
| } | ||
| else { | ||
| endpoint = `${this.resource}/providers/Microsoft.PowerApps/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`; | ||
| } |
There was a problem hiding this comment.
These endpoint URLs have a lot of common parts. Let's just use a single inline condition to add the admin scope and environment when needed (just like we do with pa app get).
There was a problem hiding this comment.
I think that single inline condition was there and it was changed according to the previous review. In pa app get I've removed the single inline condition
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
Looks good! Made a few small changes while merging.
| let endpoint; | ||
| if (args.options.asAdmin) { | ||
| endpoint = `${this.resource}/providers/Microsoft.PowerApps/scopes/admin/environments/${formatting.encodeQueryParameter(args.options.environmentName!)}/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`; | ||
| } | ||
| else { | ||
| endpoint = `${this.resource}/providers/Microsoft.PowerApps/apps/${formatting.encodeQueryParameter(args.options.name)}?api-version=2017-08-01`; | ||
| } |
There was a problem hiding this comment.
Let's isolate the common parts.
|
Merged manually, thank you for this addition! |
2 participants
Closes #5519